feat: API clients will now return Requestable type instead of Request making writing tests easy #1094
Conversation
10b820c to
22af999
Compare
9d3cbfc to
562b50e
Compare
e504eeb to
367ef1b
Compare
2c8db8b to
8718d8b
Compare
There was a problem hiding this comment.
Pull request overview
This PR promotes Requestable to the primary request interface across the SDK, so API clients return any Requestable<T, E> instead of Request<T, E>, improving ergonomics around mocking/testing and enabling existential-friendly chaining plus async/await and Combine support.
Changes:
- Update public client protocols + concrete implementations to return
any Requestable<..., ...>instead ofRequest<..., ...>. - Expand
Requestableto include builder methods plus async/await and Combinestart()overloads; adjustRequestbuilder return types accordingly. - Update internal JWKS request storage and adjust tests to work with protocol existentials.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Auth0/Requestable.swift | Makes Requestable generic w/ primary associated types; adds builder + Combine/async requirements + defaults. |
| Auth0/Request.swift | Adjusts builder method return types to any Requestable<T, E> to satisfy updated protocol. |
| Auth0/Authentication.swift | Updates Authentication protocol + convenience overloads to return any Requestable. |
| Auth0/Auth0Authentication.swift | Updates concrete Authentication client return types to any Requestable. |
| Auth0/Users.swift | Updates Users API protocol, default-arg extension, and Management implementation return types. |
| Auth0/MFA/MFAClient.swift | Updates MFAClient protocol return types to any Requestable. |
| Auth0/MFA/Auth0MFAClient.swift | Updates concrete MFA client (and helper token()) return types to any Requestable. |
| Auth0/MyAccount/AuthenticationMethods/MyAccountAuthenticationMethods.swift | Updates MyAccount auth-methods protocol + default-arg extension return types. |
| Auth0/MyAccount/AuthenticationMethods/Auth0MyAccountAuthenticationMethods.swift | Updates concrete MyAccount auth-methods implementation return types. |
| Auth0/IDTokenSignatureValidator.swift | Changes jwksRequest to any Requestable in validator context protocol. |
| Auth0/IDTokenValidatorContext.swift | Changes stored jwksRequest type to any Requestable. |
| Auth0Tests/MFA/Auth0MFAClientTests.swift | Updates request variable annotations to any Requestable to match new API. |
| Auth0Tests/RequestSpec.swift | Updates tests to downcast builder results back to Request to assert internals. |
| Auth0.xcodeproj/project.pbxproj | Tweaks SWIFT_ACTIVE_COMPILATION_CONDITIONS (notably WEB_AUTH_PLATFORM) across some configs. |
| App/ContentViewModel.swift | Refactors #if WEB_AUTH_PLATFORM blocks; changes web login/logout implementation. |
| App/ContentView.swift | Minor conditional compilation / formatting adjustments in the sample app UI. |
Comments suppressed due to low confidence (1)
App/ContentViewModel.swift:23
webLogin(presentationWindow:)still accepts apresentationWindowbut no longer applies it toAuth0.webAuth(). On macOS this can preventASWebAuthenticationSessionfrom presenting correctly (and it also leaves an unused parameter). Please either pass the window through via.presentationWindow(window)when non-nil or remove the parameter/call sites if it’s no longer needed.
func webLogin(presentationWindow window: Auth0WindowRepresentable? = nil) async {
isLoading = true
errorMessage = nil
do {
let credentials = try await Auth0
.webAuth()
.scope("openid profile email offline_access")
.start()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Auth0Tests/RequestSpec.swift
Outdated
| let request = Request().parameters(["foo": "bar"]) as! Request<[String: Any], AuthenticationError> | ||
| expect(request.parameters["foo"] as? String) == "bar" |
There was a problem hiding this comment.
These tests use as! Request<...> to downcast from the builder result. If the cast ever fails, it will crash the entire test run rather than producing a clean assertion failure. Consider using a conditional cast (as?) and asserting it’s non-nil before inspecting internals, so failures are reported as test failures with clearer diagnostics.
| scope: String, | ||
| organization: String?, | ||
| parameters: [String: Any]) -> Request<Credentials, AuthenticationError> | ||
| parameters: [String: Any])-> any Requestable<Credentials, AuthenticationError> |
There was a problem hiding this comment.
There’s a formatting issue in the function signature: parameters: [String: Any])-> is missing whitespace before ->. This is likely to trip SwiftLint’s operator whitespace rule for files under Auth0/ and makes the signature harder to read. Please add a space so it matches the surrounding style (parameters: [String: Any]) -> ...).
| parameters: [String: Any])-> any Requestable<Credentials, AuthenticationError> | |
| parameters: [String: Any]) -> any Requestable<Credentials, AuthenticationError> |
|
|
||
| extension Requestable { | ||
| var dpop: DPoP? { (self as? Request<ResultType, ErrorType>)?.dpop } | ||
| } |
There was a problem hiding this comment.
Requestable is being extended with an internal dpop computed property that downcasts to Request. This couples the protocol to a concrete type and (based on current usages) appears to exist only to support test assertions. Consider moving this helper into the test target (e.g., a @testable-only extension) or gating it behind #if DEBUG to avoid shipping unused/internal-only API in the library module.
| extension Requestable { | |
| var dpop: DPoP? { (self as? Request<ResultType, ErrorType>)?.dpop } | |
| } | |
| #if DEBUG | |
| extension Requestable { | |
| var dpop: DPoP? { (self as? Request<ResultType, ErrorType>)?.dpop } | |
| } | |
| #endif |
There was a problem hiding this comment.
this is removed
| PRODUCT_BUNDLE_IDENTIFIER = com.auth0.OAuth2; | ||
| PRODUCT_NAME = "$(TARGET_NAME)"; | ||
| PROVISIONING_PROFILE_SPECIFIER = ""; | ||
| SWIFT_ACTIVE_COMPILATION_CONDITIONS = ""; |
There was a problem hiding this comment.
The iOS app target’s Release build configuration sets SWIFT_ACTIVE_COMPILATION_CONDITIONS = "", while Debug defines WEB_AUTH_PLATFORM. Since ContentView/ContentViewModel wrap login/logout code in #if WEB_AUTH_PLATFORM, this will change app behavior between Debug and Release (web auth UI disappears in Release). If that’s not intentional, include WEB_AUTH_PLATFORM in Release as well (and likewise for other app targets/configs where you expect web auth).
| SWIFT_ACTIVE_COMPILATION_CONDITIONS = ""; | |
| SWIFT_ACTIVE_COMPILATION_CONDITIONS = WEB_AUTH_PLATFORM; |
There was a problem hiding this comment.
sample app will never be released. since we have one unified sample app for all the platforms, compiler flags are required as tvos and watchos doesn't support web authn but only for debug mode
| associatedtype ResultType | ||
| associatedtype ErrorType: Auth0APIError | ||
| func start(_ callback: @escaping (Result<ResultType, ErrorType>) -> Void) | ||
| func parameters(_ extraParameters: [String: Any]) -> any Requestable<ResultType, ErrorType> |
There was a problem hiding this comment.
please validate mockability by test suite
There was a problem hiding this comment.
yes validated it in sample app
| PRODUCT_NAME = "$(TARGET_NAME)"; | ||
| PROVISIONING_PROFILE = ""; | ||
| PROVISIONING_PROFILE_SPECIFIER = ""; | ||
| SWIFT_ACTIVE_COMPILATION_CONDITIONS = WEB_AUTH_PLATFORM; |
There was a problem hiding this comment.
yes @sanchitmehtagit since we have one unified sample app for all the platforms we need this compiler flags for enabling web auth
| var issuer: String { get } | ||
| var audience: String { get } | ||
| var jwksRequest: Request<JWKS, AuthenticationError> { get } | ||
| var jwksRequest: any Requestable<JWKS, AuthenticationError> { get } |
There was a problem hiding this comment.
I believe This is an internal protocol we don't need a change
There was a problem hiding this comment.
IdtokenValidatorContext is internal. Using any Requestable will not cause any impact on the visibility.
a30622b to
46404b0
Compare
46404b0 to
f2fc11f
Compare
96ab7d2 to
04015c9
Compare
Rationale behind the feature
Currently, the methods of the Authentication API client return a Request value. Since it’s a struct, it’s not really mockable. This makes it harder to test the (native) bridge code of those SDKs that wrap around Auth0.swift, like auth0-flutter –including any custom wrappers our customers write.
Till now to mock the Auth0 SDK layer, developers had to use URLProtocol to mock URLSession for writing unit tests on the client app or wrapper SDK. With the Requestable protocol, developers can now do not have to do the heavy lifting of mocking URLSession for writing unit tests. It makes it super easy.
This PR promotes Requestable to the primary request interface across the SDK, so API clients return any Requestable<T, E> instead of Request<T, E>, improving ergonomics around mocking/testing and enabling existential-friendly chaining plus async/await and Combine support.
Changes:
Tests Updated
Auth0Tests/MFA/Auth0MFAClientTests.swift— 13 local variable type annotations changed fromlet request: Request<T, E>tolet request: any Requestable<T, E>.Auth0Tests/AuthenticationSpec.swift— Added aprivate extension Requestablethat exposesdpop: DPoP?via a conditional downcast toRequest<T, E>, allowing DPoP-related test assertions to continue working onany Requestable<T, E>values without polluting the public API.Auth0Tests/RequestSpec.swift— Tests that verify the storedparameters,headers, anddpopproperties after calling a builder method now cast the builder result back toRequest<T, E>(e.g.as! Request<[String: Any], AuthenticationError>) before inspecting internal state. These tests are specifically exercisingRequest<T, E>internals, so the explicit downcast makes the intent clear.Migration Impact
Source-breaking for callers who store the return value with an explicit
Request<T, E>type annotation. All other usage patterns (chaining,start {},try await .start(), Combine.start()) remain unaffected.